Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Add packagePoliceDelete callback #148509

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Jan 7, 2023

Required by: #147650

This PR adds a callback that runs just before a package policy is deleted. This is needed for the APM package (and possible other use cases) where API keys embedded in the package policy will need to be deleted/invalidated so they don't linger in the system after the policy has been removed.

The new callback is called packagePoliceDelete. A delete callback already exists, however, this callback is called after the policy is deleted.

Renaming in this PR:

  • postPackagePolicyDelete -> packagePolicyPostDelete
  • DeletePackagePoliciesResponse -> PostDeletePackagePoliciesResponse
  • PostPackagePolicyDeleteCallback -> PostPackagePolicyPostDeleteCallback

@sorenlouv sorenlouv requested review from a team as code owners January 7, 2023 13:36
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 7, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes v8.7.0 labels Jan 7, 2023
@sorenlouv sorenlouv changed the title [Fleet] Add packagePolicyPostDelete and packagePoliceDelete callback [Fleet] Add packagePoliceDelete callback (rename existing packagePoliceDelete to packagePolicyPostDelete) Jan 7, 2023
@sorenlouv sorenlouv changed the title [Fleet] Add packagePoliceDelete callback (rename existing packagePoliceDelete to packagePolicyPostDelete) [Fleet] Add packagePoliceDelete callback Jan 7, 2023
@sorenlouv
Copy link
Member Author

sorenlouv commented Jan 8, 2023

@dhurley14 Can you help me understand why the security tests fail, and how to fix them?

[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu

My changes are mostly type related. Only runtime impacting change is renaming 'postPackagePolicyDelete' to 'packagePolicyPostDelete'.

Thanks!

Copy link
Contributor

@ofiriro3 ofiriro3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - cloud security posture.

@orouz Would you like to double verify me?

@nchaulet nchaulet self-requested a review January 9, 2023 12:16
Copy link
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OLM changes look good to me 👍

@dhurley14
Copy link
Contributor

@sqren it seems like those cypress tests are failing on other PR's as well.. We will look into it.

@dhurley14
Copy link
Contributor

Should be resolved once this PR is merged: #148596

I think this is a flaky cypress test (the test has a rule executing every second which, on the CI container's limited resources, could prevent kibana from loading. This seems similar to the fix here: #147349)

@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from d7afb4e to 0d18523 Compare January 9, 2023 21:47
@sorenlouv sorenlouv requested a review from a team as a code owner January 9, 2023 21:47
Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fleet changes looks good to me 🚀

@sorenlouv sorenlouv removed the request for review from joeypoon January 10, 2023 13:03
@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from 922a2de to 8e763f9 Compare January 10, 2023 13:58
@sorenlouv sorenlouv removed the request for review from a team January 10, 2023 14:00
Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sorenlouv sorenlouv enabled auto-merge (squash) January 10, 2023 15:01
@dhurley14
Copy link
Contributor

dhurley14 commented Jan 10, 2023

@sqren I think someone from @elastic/security-threat-hunting should be able to help investigate the flakiness of the other cypress tests. I'm looking into why the exceptions one is failing on your branch.

@sorenlouv
Copy link
Member Author

@sqren I think someone from @elastic/security-threat-hunting should be able to help investigate the flakiness of the other cypress tests. I'm looking into why the exceptions one is failing on your branch.

That would be great, thanks. I'll try to re-run the build but will post the failing tests here for visibility

[job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
[job] [logs] Security Solution Tests #1 / Detections : Page Filters Alert list is updated when the alerts are updated
[job] [logs] Security Solution Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar can be added notes
[job] [logs] Security Solution Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar can be added notes

@sorenlouv
Copy link
Member Author

Buildkite, test this

Add tests

Rename `PackagePolicyPostCreateCallback` back to `PostPackagePolicyPostCreateCallback`

Rename `PackagePolicyCreateCallback` back to `PostPackagePolicyCreateCallback`

Add “Post” prefix

Use `packagePolicyService.getByIDs`
@sorenlouv sorenlouv force-pushed the add-fleet-pre-delete-callback branch from 8e763f9 to 958b253 Compare January 10, 2023 17:58
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #4 / Add exception using data views from rule details Creates an exception item from alert actions overflow menu
  • [job] [logs] FTR Configs #9 / alerting api integration security and spaces enabled Alerts - Group 1 alerts find public superuser at space1 should handle find alert request appropriately
  • [job] [logs] Security Solution Tests #1 / Detections : Page Filters Alert list is updated when the alerts are updated

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 927 929 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
fleet 21 23 +2
Unknown metric groups

API count

id before after diff
fleet 1033 1035 +2

History

  • 💔 Build #99195 failed 8e763f944885adfe4dc6935a99c5df315fe50030
  • 💔 Build #98978 failed 0d18523905c05a588c6edba49e86f208acbe49b1
  • 💔 Build #98698 failed d7afb4e5c42456f8db804a4908c6bbc583a080da
  • 💔 Build #98556 failed dd6a138cff1911935b0c347d5264c4d5bc7d3145
  • 💔 Build #98544 failed 420f65634428b607af3fa4d9cd742527057e4d1c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@sorenlouv sorenlouv merged commit bd095f5 into elastic:main Jan 10, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 10, 2023
@sorenlouv sorenlouv deleted the add-fleet-pre-delete-callback branch January 10, 2023 19:55
sorenlouv added a commit that referenced this pull request Jan 11, 2023
Closes #146368
Depends on: #148509

This adds an api key to every APM package policy. This will make it
possible for APM Server to query the source map index (
`.apm-source-map` ) and the agent config index
(`.apm-agent-configuration`)
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
Closes elastic#146368
Depends on: elastic#148509

This adds an api key to every APM package policy. This will make it
possible for APM Server to query the source map index (
`.apm-source-map` ) and the agent config index
(`.apm-agent-configuration`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants